Skip to content

feat(cli): support stdin/stdout I/O using the single hyphen convention#183

Merged
pointlander merged 1 commit intopointlander:mainfrom
ardnew:cli/stdio-hyphen
Jul 27, 2025
Merged

feat(cli): support stdin/stdout I/O using the single hyphen convention#183
pointlander merged 1 commit intopointlander:mainfrom
ardnew:cli/stdio-hyphen

Conversation

@ardnew
Copy link
Copy Markdown
Contributor

@ardnew ardnew commented Jul 25, 2025

This change refactors much of main.go where command-line flags are processed.

The behavioral differences are described under their respective branches:

Condition main (current peg command) cli/stdio-hyphen (this PR)
Command-line arg is - Read input from file in CWD literally named - Read input from stdin
No command-line args given Exit with error Read input from stdin
Flag -output "-" Write parser to file in CWD literally named - Write parser to stdout
Flag -output unspecified Write parser to file with path input + ".go" Same as branch main if input is not stdin; otherwise, write parser to stdout

The usage string for flag --output was also changed from:

  -output string
        specify name of output file

to:

  -output FILE
        output to FILE ("-" for stdout)

There are no changes in all other circumstances.

Copilot AI review requested due to automatic review settings July 25, 2025 22:46
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for the standard Unix convention of using a single hyphen (-) to represent stdin/stdout in the PEG CLI tool. The changes enable reading from stdin when no arguments are provided or when - is specified as input, and writing to stdout when -output "-" is specified.

  • Refactored command-line argument processing to support stdin/stdout via hyphen convention
  • Restructured main function into smaller, more focused functions (getIO and parse)
  • Updated flag help text to clarify stdout support
Comments suppressed due to low confidence (1)

main.go:121

  • [nitpick] The parameter name compile shadows the actual purpose of this function. Consider renaming it to compileFunc or compileFn to better reflect that it's a function parameter.
	return compile(p, out)

Comment thread main.go
Comment on lines +61 to +62
}
fmt.Fprintln(os.Stderr, "warning:", err)
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic is incorrect. When *strict is true, the code calls log.Fatal(err), but when false, it prints a warning. However, this happens outside the conditional check for err != nil. The log.Fatal(err) should only be called when there's actually an error.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both logging statements are inside the conditional check for err != nil. This looks like a Copilot mistake.

Comment thread main.go
Comment on lines +83 to +85
if in != nil && in != os.Stdin {
in.Close()
}
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resource cleanup logic is duplicated between lines 83-85 and 100-102. Consider extracting this into a helper function or using defer statements to avoid code duplication.

Suggested change
if in != nil && in != os.Stdin {
in.Close()
}
closeResource(in, os.Stdin)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree with this suggestion: the reader would have to jump to another function containing only 2 lines of code and then jump back to resume reading.

So, adding closeResource is tedious for the reader and serves no functional benefit.

@pointlander pointlander merged commit 43894ca into pointlander:main Jul 27, 2025
@pointlander
Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants